-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[kube-state-metrics] set parameters for podsecurity restricted #3194
[kube-state-metrics] set parameters for podsecurity restricted #3194
Conversation
8b1d483
to
688f90d
Compare
@jcpunk, does this change require kubernetes/kube-state-metrics#2042 to be merged ? @mrueg, do you know if this is a breaking change? Not sure if the minor or major version should be bumped, but probably not patch. |
This patch does not require the upstream one to be merged. It would be nice to have up there, but not strictly required. |
I wouldn't consider this change breaking, but would prefer to raise the minor version as it changes some defaults. |
@jcpunk Can you bump the minor version instead of the patch? |
Signed-off-by: Pat Riehecky <riehecky@fnal.gov>
688f90d
to
e1b61f8
Compare
Done, and upstream has merged the non-helm permissions update. |
For me this merge causes: error validating data: [ValidationError(Deployment.spec.template.spec.securityContext): unknown field "allowPrivilegeEscalation" in io.k8s.api.core.v1.PodSecurityContext, ValidationError(Deployment.spec.template.spec.securityContext): unknown field "capabilities" in io.k8s.api.core.v1.PodSecurityContext] I believe capabilities is only on container level securitycontext |
Yes, definitely broken after the merge, seeing the same as @georgekaz |
Can you share your respective Kubernetes versions @jcpunk, @georgekaz and @atarax? Will try to look at this when I can, in the meantime you can use the previous version. |
1.24 here |
1.26 for me |
it fails on v1.24.9 as well. |
Signed-off-by: David Calvert <david.calvert@oqton.com>
it's not related to the version, like @georgekaz mentioned, capabilities is a container-level setting |
Revert is to unblock the situation, don't have time to dig into the issue right now, but this will be done later. |
In theory this fixes the bug introduced in prometheus-community#3194 Signed-off-by: Pat Riehecky <riehecky@fnal.gov>
In theory I've got another patch that does this the right way now. In practice, they both seem to install on my test cluster, so I suspect my test cluster is super messed up.... |
In theory this fixes the bug introduced in prometheus-community#3194 Signed-off-by: Pat Riehecky <riehecky@fnal.gov>
In theory this fixes the bug introduced in prometheus-community#3194 Signed-off-by: Pat Riehecky <riehecky@fnal.gov>
…theus-community#3194) Signed-off-by: Pat Riehecky <riehecky@fnal.gov> Co-authored-by: Manuel Rüger <manuel@rueg.eu>
…-community#3233) Signed-off-by: David Calvert <david.calvert@oqton.com>
What this PR does / why we need it
This permits installing this change in a namespace with PodSecurity set to
restricted
.Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged)Special notes for your reviewer
Checklist
[prometheus-couchdb-exporter]
)